refactor: stop probing emptiness with Object.keys(obj).length - #9548
Conversation
Overall package sizeSelf size: 8.2 MB Dependency sizes| name | version | self size | total size | |------|---------|-----------|------------| | import-in-the-middle | 3.3.3 | 125.43 kB | 445.14 kB | | opentracing | 0.14.7 | 194.81 kB | 194.81 kB | | dc-polyfill | 0.1.11 | 25.74 kB | 25.74 kB |🤖 This report was automatically generated by heaviest-objects-in-the-universe |
🎉 All green!🧪 All tests passed 🔄 Datadog retried 1 test - 1 passed on retry 🎯 Code Coverage (details) 🔗 Commit SHA: 3ee4385 | Docs | Datadog PR Page | Give us feedback! |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #9548 +/- ##
========================================
Coverage 98.53% 98.53%
========================================
Files 978 978
Lines 144176 144181 +5
Branches 12738 12496 -242
========================================
+ Hits 142059 142068 +9
+ Misses 2117 2113 -4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
BenchmarksBenchmark execution time: 2026-08-17 10:58:58 Comparing candidate commit 3ee4385 in PR branch Found 0 performance improvements and 0 performance regressions! Performance is the same for 2283 metrics, 9 unstable metrics.
|
8c67d50 to
7579490
Compare
There was a problem hiding this comment.
More details
Repeated and partial require-hook teardown preserved remaining registrations, while absent coverage, empty baggage, profiler metadata, and conditional OpenAI log payloads retained their expected output contracts.
📊 Validated against 6 scenarios · Open Bits AI session
🤖 Datadog Autotest · Commit e2ca12b · What is Autotest? · @DataDog review to ask questions · Any feedback? Reach out in #autotest
…e's shape
`parseSkippableSuitesResponse` leaves `coverage` undefined when the backend
sends none, and the instrumentations keep that `undefined` instead of
re-normalizing it to `{}`. Reinstating an empty-object default anywhere along
that path turns "nothing to backfill" back into a state only an `Object.keys`
probe can tell apart from real coverage.
`getConfiguredEfdRetryCount` relies on `getMaxEfdRetryCount` already returning
`undefined` for an empty bucket map, so the caller-side emptiness check folds
into the existing fallback.
…o log The extraction helpers return a store instead of filling one the caller allocated up front, so `store.openai` stays undefined for the methods that record nothing. `sendLog` keys off that: any helper that starts writing to the store must create it, or its data never reaches the log.
`SpaceProfiler` has nothing to contribute, so it returns undefined rather than an empty object the submitter has to probe before dropping it. `webTags` is only ever the tag map of a span `isWebServerSpan` matched, so it always carries `span.type`; the emptiness check in front of the endpoint label could never fire.
… aiguard 1. `ritm` counts hooked modules as they are registered and removed. The counter is the only thing deciding whether teardown resets the require patch, so every add and delete of a `moduleHooks` key has to keep it in step; `unhook` skips modules it no longer holds so a repeated teardown cannot undercount. 2. The OTel context manager builds the baggage entries in one pass and lets the absence of that object stand for "no baggage". 3. Azure Functions span links drop the probe in front of `tracer.extract`, which already returns null for a carrier that holds no trace context.
…*/src The selector only matches probe positions — a comparison against 0, a `!` operand, or the test of an `if`, ternary, logical expression or loop — so a genuine count such as `writeMapPrefix(Object.keys(x).length)` stays legal. Four sites keep the probe behind an inline opt-out. Fastify cookies and bodies and router path params reach us as third-party objects with arbitrary keys, and publishing an empty one would set the matching WAF address and burn the once-per-request publish latch. The debugger snapshot walker faces the same shape: `fields` is keyed by the user's variable names.
Pin the undefined values returned when profiler info or suite coverage is absent. Exercise legacy OpenAI edit logging through the public SDK path so the lazy store branch remains covered.
Loading nock at module scope patches HTTP before agent.load(), so OpenAI instrumentation wraps the patched methods and the remaining span assertions fail.
On Node.js 24 with representative small configs, spreading and probing takes 10.3–11.5 ns versus 17.5–20.9 ns for copying entries while tracking state.
Leaving Nock active after the legacy edit request rejects every subsequent VCR call as an unmatched request.
e2ca12b to
3ee4385
Compare
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3ee43851e3
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
`Object.keys(obj).length` allocates a full key array when callers only need to know whether data exists. Replace those probes with producer-owned presence state, `undefined` for absent results, and existing API absence signals, preserving the distinction between absent and populated data without a consumer-side allocation. A narrow `no-restricted-syntax` rule catches the pattern only in boolean/test positions, leaving genuine counts valid. Explicit exceptions remain for arbitrary user-defined keys and copy paths where separate presence tracking would change existing semantics.
`Object.keys(obj).length` allocates a full key array when callers only need to know whether data exists. Replace those probes with producer-owned presence state, `undefined` for absent results, and existing API absence signals, preserving the distinction between absent and populated data without a consumer-side allocation. A narrow `no-restricted-syntax` rule catches the pattern only in boolean/test positions, leaving genuine counts valid. Explicit exceptions remain for arbitrary user-defined keys and copy paths where separate presence tracking would change existing semantics.
`Object.keys(obj).length` allocates a full key array when callers only need to know whether data exists. Replace those probes with producer-owned presence state, `undefined` for absent results, and existing API absence signals, preserving the distinction between absent and populated data without a consumer-side allocation. A narrow `no-restricted-syntax` rule catches the pattern only in boolean/test positions, leaving genuine counts valid. Explicit exceptions remain for arbitrary user-defined keys and copy paths where separate presence tracking would change existing semantics.
`Object.keys(obj).length` allocates a full key array when callers only need to know whether data exists. Replace those probes with producer-owned presence state, `undefined` for absent results, and existing API absence signals, preserving the distinction between absent and populated data without a consumer-side allocation. A narrow `no-restricted-syntax` rule catches the pattern only in boolean/test positions, leaving genuine counts valid. Explicit exceptions remain for arbitrary user-defined keys and copy paths where separate presence tracking would change existing semantics.
Summary
Object.keys(obj).lengthallocates the whole key array to answer a yes/no question, and every site removed heresits on a hot path. Each one now answers the question from a shape that already carries the answer, and a
no-restricted-syntaxselector underpackages/*/srckeeps the probe from growing back.ritmcounts hooked modules as they are registered and removed, so teardown decides from a counter rather thanfrom the key set.
unhookskips modules it no longer holds, so a repeated teardown cannot undercount and leavethe require patch installed.
"no baggage". Azure Functions span links drop the probe in front of
tracer.extract, which already returnsnullfor a carrier without trace context.undefinedinstead of an empty object the submitter has to probe.The endpoint-label check went with it:
webTagsis only ever the tag map of a spanisWebServerSpanmatched,so it always carries
span.typeand the probe could never fire.store.openaistaysundefinedfor the methods that record nothing.Object.assignwould claimmetadata exists even when the source object is empty.
coverageand the EFD retry bucket mapundefinedwhen the backend sends nothing, sono caller has to tell an empty object apart from real data.
Why the rule is narrow
The selector only matches probe positions — a comparison against
0, a!operand, or the test of anif,ternary, logical expression or loop — so a genuine count such as
writeMapPrefix(Object.keys(x).length)stayslegal.
Four sites keep the probe behind an inline opt-out. Fastify cookies and bodies and router path params reach us as
third-party objects with arbitrary keys, where publishing an empty one would set the matching WAF address and burn
the once-per-request publish latch. The debugger snapshot walker faces the same shape:
fieldsis keyed by theuser's variable names.